chart, salt: Add annotations to fluent-bit pod by eg-ayoub · Pull Request #4834 · scality/metalk8s

eg-ayoub · 2026-03-23T11:07:03Z

What/How: Add annotations containing configmap and secret to fluent-bit pod
Why: so it is restarted whenever these objects are updated.

MK8S-203

bert-e · 2026-03-23T11:07:08Z

Hello eg-ayoub,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2026-03-23T11:07:20Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

charts/fluent-bit.yaml

CHANGELOG.md

claude · 2026-03-24T14:31:32Z

CHANGELOG.md: Typo — "it's" should be "its" (possessive form)
- Replace "it's configmap" with "its configmap"

Review by Claude Code

claude · 2026-03-24T14:37:07Z

LGTM

The changes correctly replace the static (and useless) checksum annotation and the prometheus annotations with dynamic Salt slot-based digests for the ConfigMap and Secret, following the same established pattern used in calico and kube-proxy. The data: {} addition to the initial Secret manifest ensures get_object_digest(path="data") won't fail on first deployment. Prometheus scraping is already covered by the ServiceMonitor, so removing the prometheus.io/* annotations has no functional impact.

Review by Claude Code

bert-e · 2026-03-25T08:48:35Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

Peer approvals must include at least 1 approval from the following list:

claude · 2026-03-25T08:50:14Z

LGTM

Review by Claude Code

salt/metalk8s/addons/logging/fluent-bit/deployed/chart.sls

charts/fluent-bit.yaml

salt/_modules/metalk8s_kubernetes.py

claude · 2026-03-25T13:19:51Z

chart.sls out of sync with values file: checksum/secret slot call in chart.sls is missing allow_empty=True that is present in charts/fluent-bit.yaml
- Regenerate chart.sls from the Helm chart with the updated values
- Secret checksum hashes entire object instead of data: Without a path parameter, the secret digest includes mutable metadata (resourceVersion, timestamps), causing unnecessary pod restarts on every reconciliation even when secret data is unchanged
- Add path="data" to the secret checksum call, consistent with other checksums in the codebase (dex, alertmanager, loki all use specific paths)
- Missing test coverage for allow_empty parameter: No unit test verifies the new allow_empty=True behavior in get_object_digest
- Add a test case in test_metalk8s_kubernetes.yaml with allow_empty=True, an invalid path, and expected result of empty string

Review by Claude Code

charts/fluent-bit.yaml

salt/_modules/metalk8s_kubernetes.py

claude · 2026-03-25T13:24:35Z

checksum/secret hashes the entire Secret object (including metadata like resourceVersion) because no path parameter is passed to get_object_digest. This will cause fluent-bit pods to restart on every Salt run, not just when secret data changes.
- Pass path="data" to hash only the secret's data payload, consistent with all other get_object_digest usages in the codebase.
- No test coverage for the new allow_empty parameter in get_object_digest.
- Add test cases in salt/tests/unit/modules/files/test_metalk8s_kubernetes.yaml for both allow_empty=True (returns empty string) and allow_empty=False (raises error).

Review by Claude Code

salt/_modules/metalk8s_kubernetes.py

claude · 2026-03-25T13:32:00Z

allow_empty=True suppresses errors for both empty data and completely missing keys, which could silently mask typos in path arguments. Consider adding a test case with a non-existent path and allow_empty=True to document this behavior.

Review by Claude Code

claude · 2026-03-25T14:04:58Z

LGTM

Review by Claude Code

salt/_modules/metalk8s_kubernetes.py

claude · 2026-03-25T16:12:21Z

ignore_not_found parameter name is ambiguous — it only handles missing paths within an object, not a missing object itself. Consider renaming to ignore_missing_path or extending the behavior to also cover object-level 404s.

Review by Claude Code

salt/_modules/metalk8s_kubernetes.py

Add annotations containing configmap and secret to fluent-bit pod so it is restarted whenever these objects are updated. MK8S-203

claude · 2026-03-26T08:37:35Z

salt/_modules/metalk8s_kubernetes.py


-def get_object_digest(path=None, checksum="sha256", *args, **kwargs):
+def get_object_digest(
+    path=None, checksum="sha256", ignore_not_found=False, *args, **kwargs


`ignore_not_found` only guards against the path not being found within an existing object. If the Secret itself does not exist (`get_object` returns `None`), line 431 still raises `CommandExecutionError("Unable to find the object")` regardless of `ignore_not_found`.

This works today because Salt include ordering in `init.sls` ensures `.secret` runs before `.chart`. But the parameter name is misleading — consider either:
1. Also applying `ignore_not_found` to the object-level check (line 430-431), or
2. Renaming the parameter to something like `ignore_empty_path` to clarify its scope.

— Claude Code

claude · 2026-03-26T08:37:36Z

salt/tests/unit/modules/files/test_metalk8s_kubernetes.yaml

+      data: {}
+    path: 'daat'
+    ignore_not_found: True
+    result: ''


Tests cover `ignore_not_found=True` when the path is empty or wrong, but there is no test for `ignore_not_found=True` when the object itself is `None` (`obj: null`). Adding one would document the current behavior (raises error) and protect against unintended changes.

— Claude Code

claude · 2026-03-26T08:37:45Z

ignore_not_found parameter in get_object_digest only handles path-not-found within an existing object, not the case where the object itself is missing. The name is misleading — consider renaming to ignore_empty_path or extending it to also cover object-level 404s.
- Rename the parameter, or also guard the object-level check at line 430-431
Missing test case for ignore_not_found=True when obj is null
- Add a test with obj: null and ignore_not_found: True to document that it still raises

Review by Claude Code

eg-ayoub · 2026-03-26T16:39:07Z

/approve

bert-e · 2026-03-26T16:39:25Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/133.0

The following branches will NOT be impacted:

development/123.0
development/124.0
development/124.1
development/125.0
development/126.0
development/127.0
development/128.0
development/129.0
development/130.0
development/131.0
development/132.0
development/2.0
development/2.1
development/2.10
development/2.11
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2026-03-26T19:03:10Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/133.0

The following branches have NOT changed:

development/123.0
development/124.0
development/124.1
development/125.0
development/126.0
development/127.0
development/128.0
development/129.0
development/130.0
development/131.0
development/132.0
development/2.0
development/2.1
development/2.10
development/2.11
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

Please check the status of the associated issue None.

Goodbye eg-ayoub.

eg-ayoub requested a review from a team as a code owner March 23, 2026 11:07

TeddyAndrieux previously approved these changes Mar 23, 2026

View reviewed changes

charts/fluent-bit.yaml Outdated Show resolved Hide resolved

eg-ayoub dismissed TeddyAndrieux’s stale review via 2164dbc March 23, 2026 13:24

TeddyAndrieux previously approved these changes Mar 23, 2026

View reviewed changes

anthony-treuillier-scality previously approved these changes Mar 24, 2026

View reviewed changes

eg-ayoub dismissed stale reviews from anthony-treuillier-scality and TeddyAndrieux via e5434b1 March 24, 2026 08:45

eg-ayoub force-pushed the improvement/ensure-fluent-bit-restarted branch 2 times, most recently from e5434b1 to 6ca6b22 Compare March 24, 2026 14:29

claude bot reviewed Mar 24, 2026

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

eg-ayoub force-pushed the improvement/ensure-fluent-bit-restarted branch from 6ca6b22 to 87c360c Compare March 24, 2026 14:35

eg-ayoub force-pushed the improvement/ensure-fluent-bit-restarted branch from 87c360c to 1bf14b6 Compare March 25, 2026 08:48